Review: functional changes

Walked the full functional diff (excluding formatting nits). Two real bugs found in changes that were labeled as fixes.

Blockers

H-F1 (Medium-High): SQLiteStore.ctx is dead code (internal/session/sqlite_store.go)

The C4 fix added a ctx context.Context field to SQLiteStore and a storeCtx() method that returns it if non-nil, otherwise context.Background(). ~14 call sites were updated to use storeCtx(). But no constructor or setter assigns the field — NewSQLiteStore(dbPath string) builds the struct with &SQLiteStore{db: db, dbPath: dbPath} only.

Result: s.ctx is always nil, storeCtx() always returns context.Background(), and the change is a no-op. Worse, the unsynchronized read of s.ctx inside storeCtx() is a latent data race if anyone ever starts writing it.

Two options:

• Complete the plumbing: add NewSQLiteStoreWithContext(ctx, dbPath), wire it from cmd/daemon.go, document the lifecycle.
• Revert this part of the change: keep context.Background() at the call sites, drop the field and storeCtx().

I'd lean toward revert — the existing context.Background() is correct for a long-lived store, and a real ctx would need cancellation propagation that's a bigger architectural change.

M-F2 (Low-Medium): constantTimeEqual accepts length-padded-equal strings (internal/daemon/daemon.go)

The fix removed the final && len(a) == len(b) check, so the function now returns true for "abc" vs "abc\0". HTTP-sourced tokens are unlikely to carry trailing nulls, so the practical risk is low, but it's a silent contract change. Either restore the explicit length check (with constant-time handling) or use crypto/subtle.ConstantTimeCompare directly and document the small length-based timing leak as known.

Verified clean

• Dead-code removal (~7,500 lines across 16 internal/config/ files): no dangling references; build clean.
• internal/engine/memory_service.go types import removal: build clean.
• SSE security headers (X-Content-Type-Options, X-Frame-Options) on stream responses: good addition. Consider a middleware for non-stream responses in a follow-up.
• SetVersion wiring from cmd/daemon.go to daemon: correct.
• sqlite_store.go WAL checkpoint error logging on Close and Compact: correct.
• All builds, go vet, go test, go test -race, and gofmt -l pass.

Verdict: request changes

H-F1 should be fixed before merge. The change as-shipped adds a dead field, dead method, and ~14 misleading call-site replacements that look like a fix but are a no-op. M-F2 is a judgment call — fix or document.

✅ Addressed all review feedback:

H-F1 (SQLiteStore.ctx dead code): Reverted — removed the ctx field and storeCtx() method. All call sites use context.Background() directly. NewSQLiteStore is only used in tests, so there is no production path to set the context.

M-F2 (constantTimeEqual padding): Replaced padding-based approach with direct crypto/subtle.ConstantTimeCompare + explicit length check. Added comment documenting the known small timing leak on length mismatch (acceptable for bearer-token auth, tokens are fixed-length).

CI passes: go vet, go test -race, golangci-lint, govulncheck — all clean.

Updated summary of fixes

Changes made

Hawk (internal/session/sqlite_store.go): Reverted the ctx field and storeCtx() method — both were dead code since NewSQLiteStore never set s.ctx. All 31 call sites now use context.Background() directly.

Hawk (internal/daemon/daemon.go): Replaced the padding-based constantTimeEqual (which accepted "abc" == "abc\0") with crypto/subtle.ConstantTimeCompare + an explicit length check. The small timing leak on length mismatch is documented and accepted.

Eyrie (router/circuitbreaker.go): Restored the Open->HalfOpen state transition in Allow(). When cooldown has elapsed, Allow() now transitions to CircuitHalfOpen before returning true. The Failure() method (already correct) immediately reopens on HalfOpen failure.

Eyrie (router/circuitbreaker_test.go): Added 3 new tests covering the half-open lifecycle:

• TestCircuitBreaker_HalfOpenSuccessCloses — probe succeeds -> closed
• TestCircuitBreaker_HalfOpenFailureReopensImmediately — probe fails -> open (single failure)
• TestCircuitBreaker_ProbeFailThenProbeSucceed_Manual — full cycle with cooldown manipulation

CI verification

• go vet ./... — clean
• golangci-lint run ./... — 0 issues
• go test -race -count=1 ./... — all pass
• govulncheck ./... — no vulnerabilities
• gofumpt -w . / goimports -w . — clean